From 9f159dd62cbb1a4efe9c5cd724a94caf8c885793 Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 19 Feb 2024 09:47:19 -0500 Subject: nvnflinger/vi: don't recreate buffer queue on open/close --- src/core/hle/service/nvnflinger/display.h | 26 +++++------ .../hle/service/nvnflinger/hardware_composer.cpp | 12 ++--- .../hle/service/nvnflinger/surface_flinger.cpp | 39 +++++++++++----- src/core/hle/service/nvnflinger/surface_flinger.h | 6 ++- src/core/hle/service/vi/container.cpp | 52 +++++++++++----------- src/core/hle/service/vi/container.h | 3 -- src/core/hle/service/vi/layer.h | 16 ++++--- src/core/hle/service/vi/layer_list.h | 6 ++- 8 files changed, 88 insertions(+), 72 deletions(-) diff --git a/src/core/hle/service/nvnflinger/display.h b/src/core/hle/service/nvnflinger/display.h index f27cbf144..40aa59787 100644 --- a/src/core/hle/service/nvnflinger/display.h +++ b/src/core/hle/service/nvnflinger/display.h @@ -3,8 +3,6 @@ #pragma once -#include - #include "core/hle/service/nvnflinger/buffer_item_consumer.h" #include "core/hle/service/nvnflinger/hwc_layer.h" @@ -26,18 +24,12 @@ struct Layer { }; struct LayerStack { - std::list layers; -}; - -struct Display { - explicit Display(u64 id_) { - id = id_; - } + std::vector> layers; - Layer* FindLayer(s32 consumer_id) { - for (auto& layer : stack.layers) { - if (layer.consumer_id == consumer_id) { - return &layer; + std::shared_ptr FindLayer(s32 consumer_id) { + for (auto& layer : layers) { + if (layer->consumer_id == consumer_id) { + return layer; } } @@ -45,7 +37,13 @@ struct Display { } bool HasLayers() { - return !stack.layers.empty(); + return !layers.empty(); + } +}; + +struct Display { + explicit Display(u64 id_) { + id = id_; } u64 id; diff --git a/src/core/hle/service/nvnflinger/hardware_composer.cpp b/src/core/hle/service/nvnflinger/hardware_composer.cpp index 02215a786..f2dfe85a9 100644 --- a/src/core/hle/service/nvnflinger/hardware_composer.cpp +++ b/src/core/hle/service/nvnflinger/hardware_composer.cpp @@ -55,10 +55,10 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display, // Acquire all necessary framebuffers. for (auto& layer : display.stack.layers) { - auto consumer_id = layer.consumer_id; + auto consumer_id = layer->consumer_id; // Try to fetch the framebuffer (either new or stale). - const auto result = this->CacheFramebufferLocked(layer, consumer_id); + const auto result = this->CacheFramebufferLocked(*layer, consumer_id); // If we failed, skip this layer. if (result == CacheStatus::NoBufferAvailable) { @@ -75,7 +75,7 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display, const auto& igbp_buffer = *item.graphic_buffer; // TODO: get proper Z-index from layer - if (layer.visible) { + if (layer->visible) { composition_stack.emplace_back(HwcLayer{ .buffer_handle = igbp_buffer.BufferId(), .offset = igbp_buffer.Offset(), @@ -84,7 +84,7 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display, .height = igbp_buffer.Height(), .stride = igbp_buffer.Stride(), .z_index = 0, - .blending = layer.blending, + .blending = layer->blending, .transform = static_cast(item.transform), .crop_rect = item.crop, .acquire_fence = item.fence, @@ -134,7 +134,7 @@ u32 HardwareComposer::ComposeLocked(f32* out_speed_scale, Display& display, continue; } - if (auto* layer = display.FindLayer(layer_id); layer != nullptr) { + if (const auto layer = display.stack.FindLayer(layer_id); layer != nullptr) { // TODO: support release fence // This is needed to prevent screen tearing layer->buffer_item_consumer->ReleaseBuffer(framebuffer.item, android::Fence::NoFence()); @@ -153,7 +153,7 @@ void HardwareComposer::RemoveLayerLocked(Display& display, ConsumerId consumer_i } // Try to release the buffer item. - auto* const layer = display.FindLayer(consumer_id); + const auto layer = display.stack.FindLayer(consumer_id); if (layer && it->second.is_acquired) { layer->buffer_item_consumer->ReleaseBuffer(it->second.item, android::Fence::NoFence()); } diff --git a/src/core/hle/service/nvnflinger/surface_flinger.cpp b/src/core/hle/service/nvnflinger/surface_flinger.cpp index 41a705717..8362b65e5 100644 --- a/src/core/hle/service/nvnflinger/surface_flinger.cpp +++ b/src/core/hle/service/nvnflinger/surface_flinger.cpp @@ -36,7 +36,7 @@ void SurfaceFlinger::RemoveDisplay(u64 display_id) { bool SurfaceFlinger::ComposeDisplay(s32* out_swap_interval, f32* out_compose_speed_scale, u64 display_id) { auto* const display = this->FindDisplay(display_id); - if (!display || !display->HasLayers()) { + if (!display || !display->stack.HasLayers()) { return false; } @@ -46,19 +46,34 @@ bool SurfaceFlinger::ComposeDisplay(s32* out_swap_interval, f32* out_compose_spe return true; } -void SurfaceFlinger::AddLayerToDisplayStack(u64 display_id, s32 consumer_binder_id) { - auto* const display = this->FindDisplay(display_id); +void SurfaceFlinger::CreateLayer(s32 consumer_binder_id) { auto binder = std::static_pointer_cast( m_server.TryGetBinder(consumer_binder_id)); - - if (!display || !binder) { + if (!binder) { return; } auto buffer_item_consumer = std::make_shared(std::move(binder)); buffer_item_consumer->Connect(false); - display->stack.layers.emplace_back(std::move(buffer_item_consumer), consumer_binder_id); + m_layers.layers.emplace_back( + std::make_shared(std::move(buffer_item_consumer), consumer_binder_id)); +} + +void SurfaceFlinger::DestroyLayer(s32 consumer_binder_id) { + std::erase_if(m_layers.layers, + [&](auto& layer) { return layer->consumer_id == consumer_binder_id; }); +} + +void SurfaceFlinger::AddLayerToDisplayStack(u64 display_id, s32 consumer_binder_id) { + auto* const display = this->FindDisplay(display_id); + auto layer = this->FindLayer(consumer_binder_id); + + if (!display || !layer) { + return; + } + + display->stack.layers.emplace_back(std::move(layer)); } void SurfaceFlinger::RemoveLayerFromDisplayStack(u64 display_id, s32 consumer_binder_id) { @@ -69,18 +84,18 @@ void SurfaceFlinger::RemoveLayerFromDisplayStack(u64 display_id, s32 consumer_bi m_composer.RemoveLayerLocked(*display, consumer_binder_id); std::erase_if(display->stack.layers, - [&](auto& layer) { return layer.consumer_id == consumer_binder_id; }); + [&](auto& layer) { return layer->consumer_id == consumer_binder_id; }); } void SurfaceFlinger::SetLayerVisibility(s32 consumer_binder_id, bool visible) { - if (auto* layer = this->FindLayer(consumer_binder_id); layer != nullptr) { + if (const auto layer = this->FindLayer(consumer_binder_id); layer != nullptr) { layer->visible = visible; return; } } void SurfaceFlinger::SetLayerBlending(s32 consumer_binder_id, LayerBlending blending) { - if (auto* layer = this->FindLayer(consumer_binder_id); layer != nullptr) { + if (const auto layer = this->FindLayer(consumer_binder_id); layer != nullptr) { layer->blending = blending; return; } @@ -96,9 +111,9 @@ Display* SurfaceFlinger::FindDisplay(u64 display_id) { return nullptr; } -Layer* SurfaceFlinger::FindLayer(s32 consumer_binder_id) { - for (auto& display : m_displays) { - if (auto* layer = display.FindLayer(consumer_binder_id); layer != nullptr) { +std::shared_ptr SurfaceFlinger::FindLayer(s32 consumer_binder_id) { + for (auto& layer : m_layers.layers) { + if (layer->consumer_id == consumer_binder_id) { return layer; } } diff --git a/src/core/hle/service/nvnflinger/surface_flinger.h b/src/core/hle/service/nvnflinger/surface_flinger.h index d8c53fbda..406281c83 100644 --- a/src/core/hle/service/nvnflinger/surface_flinger.h +++ b/src/core/hle/service/nvnflinger/surface_flinger.h @@ -36,6 +36,9 @@ public: void RemoveDisplay(u64 display_id); bool ComposeDisplay(s32* out_swap_interval, f32* out_compose_speed_scale, u64 display_id); + void CreateLayer(s32 consumer_binder_id); + void DestroyLayer(s32 consumer_binder_id); + void AddLayerToDisplayStack(u64 display_id, s32 consumer_binder_id); void RemoveLayerFromDisplayStack(u64 display_id, s32 consumer_binder_id); @@ -44,7 +47,7 @@ public: private: Display* FindDisplay(u64 display_id); - Layer* FindLayer(s32 consumer_binder_id); + std::shared_ptr FindLayer(s32 consumer_binder_id); public: // TODO: these don't belong here @@ -57,6 +60,7 @@ private: KernelHelpers::ServiceContext m_context; std::vector m_displays; + LayerStack m_layers; std::shared_ptr nvdrv; s32 disp_fd; HardwareComposer m_composer; diff --git a/src/core/hle/service/vi/container.cpp b/src/core/hle/service/vi/container.cpp index 310a207f1..9074f4ae0 100644 --- a/src/core/hle/service/vi/container.cpp +++ b/src/core/hle/service/vi/container.cpp @@ -43,11 +43,7 @@ void Container::OnTerminate() { m_is_shut_down = true; - m_layers.ForEachLayer([&](auto& layer) { - if (layer.IsOpen()) { - this->DestroyBufferQueueLocked(&layer); - } - }); + m_layers.ForEachLayer([&](auto& layer) { this->DestroyLayerLocked(layer.GetId()); }); m_displays.ForEachDisplay( [&](auto& display) { m_surface_flinger->RemoveDisplay(display.GetId()); }); @@ -161,16 +157,29 @@ Result Container::CreateLayerLocked(u64* out_layer_id, u64 display_id, u64 owner auto* const display = m_displays.GetDisplayById(display_id); R_UNLESS(display != nullptr, VI::ResultNotFound); - auto* const layer = m_layers.CreateLayer(owner_aruid, display); + s32 consumer_binder_id, producer_binder_id; + m_surface_flinger->CreateBufferQueue(&consumer_binder_id, &producer_binder_id); + + auto* const layer = + m_layers.CreateLayer(owner_aruid, display, consumer_binder_id, producer_binder_id); R_UNLESS(layer != nullptr, VI::ResultNotFound); + m_surface_flinger->CreateLayer(consumer_binder_id); + *out_layer_id = layer->GetId(); R_SUCCEED(); } Result Container::DestroyLayerLocked(u64 layer_id) { - R_SUCCEED_IF(m_layers.DestroyLayer(layer_id)); - R_THROW(VI::ResultNotFound); + auto* const layer = m_layers.GetLayerById(layer_id); + R_UNLESS(layer != nullptr, VI::ResultNotFound); + + m_surface_flinger->DestroyLayer(layer->GetConsumerBinderId()); + m_surface_flinger->DestroyBufferQueue(layer->GetConsumerBinderId(), + layer->GetProducerBinderId()); + m_layers.DestroyLayer(layer_id); + + R_SUCCEED(); } Result Container::OpenLayerLocked(s32* out_producer_binder_id, u64 layer_id, u64 aruid) { @@ -181,7 +190,12 @@ Result Container::OpenLayerLocked(s32* out_producer_binder_id, u64 layer_id, u64 R_UNLESS(!layer->IsOpen(), VI::ResultOperationFailed); R_UNLESS(layer->GetOwnerAruid() == aruid, VI::ResultPermissionDenied); - this->CreateBufferQueueLocked(layer); + layer->Open(); + + if (auto* display = layer->GetDisplay(); display != nullptr) { + m_surface_flinger->AddLayerToDisplayStack(display->GetId(), layer->GetConsumerBinderId()); + } + *out_producer_binder_id = layer->GetProducerBinderId(); R_SUCCEED(); @@ -192,30 +206,14 @@ Result Container::CloseLayerLocked(u64 layer_id) { R_UNLESS(layer != nullptr, VI::ResultNotFound); R_UNLESS(layer->IsOpen(), VI::ResultOperationFailed); - this->DestroyBufferQueueLocked(layer); - - R_SUCCEED(); -} - -void Container::CreateBufferQueueLocked(Layer* layer) { - s32 consumer_binder_id, producer_binder_id; - m_surface_flinger->CreateBufferQueue(&consumer_binder_id, &producer_binder_id); - layer->Open(consumer_binder_id, producer_binder_id); - - if (auto* display = layer->GetDisplay(); display != nullptr) { - m_surface_flinger->AddLayerToDisplayStack(display->GetId(), consumer_binder_id); - } -} - -void Container::DestroyBufferQueueLocked(Layer* layer) { if (auto* display = layer->GetDisplay(); display != nullptr) { m_surface_flinger->RemoveLayerFromDisplayStack(display->GetId(), layer->GetConsumerBinderId()); } layer->Close(); - m_surface_flinger->DestroyBufferQueue(layer->GetConsumerBinderId(), - layer->GetProducerBinderId()); + + R_SUCCEED(); } bool Container::ComposeOnDisplay(s32* out_swap_interval, f32* out_compose_speed_scale, diff --git a/src/core/hle/service/vi/container.h b/src/core/hle/service/vi/container.h index cd0d2ca86..5eac4d77d 100644 --- a/src/core/hle/service/vi/container.h +++ b/src/core/hle/service/vi/container.h @@ -72,9 +72,6 @@ private: Result OpenLayerLocked(s32* out_producer_binder_id, u64 layer_id, u64 aruid); Result CloseLayerLocked(u64 layer_id); - void CreateBufferQueueLocked(Layer* layer); - void DestroyBufferQueueLocked(Layer* layer); - public: bool ComposeOnDisplay(s32* out_swap_interval, f32* out_compose_speed_scale, u64 display_id); diff --git a/src/core/hle/service/vi/layer.h b/src/core/hle/service/vi/layer.h index b85c8df61..e4c9c9864 100644 --- a/src/core/hle/service/vi/layer.h +++ b/src/core/hle/service/vi/layer.h @@ -13,29 +13,31 @@ class Layer { public: constexpr Layer() = default; - void Initialize(u64 id, u64 owner_aruid, Display* display) { + void Initialize(u64 id, u64 owner_aruid, Display* display, s32 consumer_binder_id, + s32 producer_binder_id) { m_id = id; m_owner_aruid = owner_aruid; m_display = display; + m_consumer_binder_id = consumer_binder_id; + m_producer_binder_id = producer_binder_id; m_is_initialized = true; } void Finalize() { m_id = {}; + m_owner_aruid = {}; m_display = {}; + m_consumer_binder_id = {}; + m_producer_binder_id = {}; m_is_initialized = {}; } - void Open(s32 consumer_binder_id, s32 producer_binder_id) { - m_consumer_binder_id = consumer_binder_id; - m_producer_binder_id = producer_binder_id; + void Open() { m_is_open = true; } void Close() { - m_producer_binder_id = {}; - m_consumer_binder_id = {}; - m_is_open = {}; + m_is_open = false; } u64 GetId() const { diff --git a/src/core/hle/service/vi/layer_list.h b/src/core/hle/service/vi/layer_list.h index 1738ede9a..4afca6f40 100644 --- a/src/core/hle/service/vi/layer_list.h +++ b/src/core/hle/service/vi/layer_list.h @@ -11,13 +11,15 @@ class LayerList { public: constexpr LayerList() = default; - Layer* CreateLayer(u64 owner_aruid, Display* display) { + Layer* CreateLayer(u64 owner_aruid, Display* display, s32 consumer_binder_id, + s32 producer_binder_id) { Layer* const layer = GetFreeLayer(); if (!layer) { return nullptr; } - layer->Initialize(++m_next_id, owner_aruid, display); + layer->Initialize(++m_next_id, owner_aruid, display, consumer_binder_id, + producer_binder_id); return layer; } -- cgit v1.2.3 From 668ff0db3a436e97578cc29ae8b783a58aedb237 Mon Sep 17 00:00:00 2001 From: Liam Date: Mon, 19 Feb 2024 09:50:33 -0500 Subject: vi: remove superfluous locking in shared buffer manager --- src/core/hle/service/vi/shared_buffer_manager.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/core/hle/service/vi/shared_buffer_manager.cpp b/src/core/hle/service/vi/shared_buffer_manager.cpp index 869b18961..3c0507d30 100644 --- a/src/core/hle/service/vi/shared_buffer_manager.cpp +++ b/src/core/hle/service/vi/shared_buffer_manager.cpp @@ -322,8 +322,6 @@ Result SharedBufferManager::GetSharedBufferMemoryHandleId(u64* out_buffer_size, Result SharedBufferManager::AcquireSharedFrameBuffer(android::Fence* out_fence, std::array& out_slot_indexes, s64* out_target_slot, u64 layer_id) { - std::scoped_lock lk{m_guard}; - // Get the producer. std::shared_ptr producer; R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id)); @@ -347,8 +345,6 @@ Result SharedBufferManager::PresentSharedFrameBuffer(android::Fence fence, Common::Rectangle crop_region, u32 transform, s32 swap_interval, u64 layer_id, s64 slot) { - std::scoped_lock lk{m_guard}; - // Get the producer. std::shared_ptr producer; R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id)); @@ -379,8 +375,6 @@ Result SharedBufferManager::PresentSharedFrameBuffer(android::Fence fence, } Result SharedBufferManager::CancelSharedFrameBuffer(u64 layer_id, s64 slot) { - std::scoped_lock lk{m_guard}; - // Get the producer. std::shared_ptr producer; R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id)); @@ -394,8 +388,6 @@ Result SharedBufferManager::CancelSharedFrameBuffer(u64 layer_id, s64 slot) { Result SharedBufferManager::GetSharedFrameBufferAcquirableEvent(Kernel::KReadableEvent** out_event, u64 layer_id) { - std::scoped_lock lk{m_guard}; - // Get the producer. std::shared_ptr producer; R_TRY(m_container.GetLayerProducerHandle(std::addressof(producer), layer_id)); -- cgit v1.2.3 From 5f7608a7c6ca7f5bf91d456f26fe40b9ef6d89e7 Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 20 Feb 2024 00:02:56 -0500 Subject: vi: ignore shared buffer destruction failure on termination --- src/core/hle/service/vi/shared_buffer_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/hle/service/vi/shared_buffer_manager.cpp b/src/core/hle/service/vi/shared_buffer_manager.cpp index 3c0507d30..12cba16fa 100644 --- a/src/core/hle/service/vi/shared_buffer_manager.cpp +++ b/src/core/hle/service/vi/shared_buffer_manager.cpp @@ -285,7 +285,7 @@ void SharedBufferManager::DestroySession(Kernel::KProcess* owner_process) { auto& session = it->second; // Destroy the layer. - R_ASSERT(m_container.DestroyStrayLayer(session.layer_id)); + m_container.DestroyStrayLayer(session.layer_id); // Close nvmap handle. FreeHandle(session.buffer_nvmap_handle, *m_nvdrv, session.nvmap_fd); -- cgit v1.2.3